Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various cleanups and speedups for the Vhosts plugin #13

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Various cleanups and speedups for the Vhosts plugin #13

wants to merge 10 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 4, 2012

There's a few changes here but each change is in a separate commit.. Other than one pure cleanup commit, the others are as follows (copied from CHANGELOG):

  • Do vhost twiddling after all host header cleanups in order to standardize with regular vhost configuration.
  • Fix handling of a www.exammple.com.:80 type host header in the vhost plugin.
  • Don't check for a vhost twiddle request_uri if twiddling hasn't been configured, so that /__using/... uris aren't needlessly appropriated.
  • Return an error rather than silently continuing when a vhost maps to a service that doesn't exist.
  • Decrease overall CPU usage by 20% when using the vhost plugin.
  • Allow vhost mappings to be deleted via the management port.

I'm not a perl programmer but even I know the vhost_selector optimization isn't very perl. Turning the closure into a static function was a lot faster (probably around 15%) but taking out the function call altogether is faster again. Yes, I could probably use a do { } while (0); and break out of it if need be, but I think gotos are more legible in this case... Just pre-empting the "WTF?!" ;)

Everything else should be pretty straight forward.

@ghost
Copy link
Author

ghost commented Feb 6, 2012

I removed the "Decrease overall CPU usage by 20% when using the vhost plugin." line as doing performance testing on my development box showed it to be more like 3%. The big drop in CPU usage on the live server must have been from something else..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant